Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move shared JavaScript to lib folder #1481

Merged
merged 8 commits into from
Aug 3, 2022
Merged

Move shared JavaScript to lib folder #1481

merged 8 commits into from
Aug 3, 2022

Conversation

lfdebrux
Copy link
Member

@lfdebrux lfdebrux commented Jul 22, 2022

Ideally, a user would just add their javascript to app/assets/javascripts/application.js and not have to see or worry about things like intitialsing govuk-frontend, or the auto-store data JS. This would make updating a user's prototype easier and less risky, because currently it is possible they have changed the standard files. It will also mean less is needed to bootstrap a prototype using a package.

This PR moves the standard JS to the lib folder as lib/assets/javascripts/kit.js, and replaces app/assets/javascripts/application.js with a simple placeholder that a user can rewrite completely as desired.

Note that we make sure that all pages include both scripts by adding kit.js to app/views/includes/scripts.html. This PR doesn't help users update that file, but ideally in another PR we would make it so that updating to v13 also takes care of this.

Resolves #1387.

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-protot-ldeb-move--5h2aj9 July 22, 2022 15:22 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-protot-ldeb-move--5h2aj9 July 22, 2022 15:27 Inactive
@lfdebrux lfdebrux force-pushed the ldeb-move-js-to-lib branch from b835956 to 7dc024e Compare July 22, 2022 15:39
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-protot-ldeb-move--5h2aj9 July 22, 2022 15:39 Inactive
@lfdebrux lfdebrux force-pushed the ldeb-move-js-to-lib branch from 7dc024e to 98e67e7 Compare July 22, 2022 16:09
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-protot-ldeb-move--5h2aj9 July 22, 2022 16:09 Inactive
@lfdebrux lfdebrux force-pushed the ldeb-move-js-to-lib branch from 98e67e7 to ab960f3 Compare July 22, 2022 16:13
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-protot-ldeb-move--5h2aj9 July 22, 2022 16:14 Inactive
@lfdebrux lfdebrux force-pushed the ldeb-move-js-to-lib branch from ab960f3 to 65d4b62 Compare July 25, 2022 10:23
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-protot-ldeb-move--5h2aj9 July 25, 2022 10:23 Inactive
@lfdebrux lfdebrux force-pushed the ldeb-move-js-to-lib branch from 65d4b62 to 29c135d Compare July 25, 2022 10:38
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-protot-ldeb-move--5h2aj9 July 25, 2022 10:39 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-protot-ldeb-move--5h2aj9 July 25, 2022 16:24 Inactive
@lfdebrux lfdebrux force-pushed the ldeb-move-js-to-lib branch from 5f7d6f9 to f9c61d7 Compare July 26, 2022 08:16
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-protot-ldeb-move--5h2aj9 July 26, 2022 08:17 Inactive
@lfdebrux lfdebrux force-pushed the ldeb-move-js-to-lib branch from f9c61d7 to 3f87634 Compare July 26, 2022 08:18
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-protot-ldeb-move--5h2aj9 July 26, 2022 08:18 Inactive
@lfdebrux lfdebrux force-pushed the ldeb-move-js-to-lib branch from 3f87634 to 99cd145 Compare July 26, 2022 08:20
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-protot-ldeb-move--5h2aj9 July 26, 2022 08:21 Inactive
@lfdebrux lfdebrux force-pushed the ldeb-move-js-to-lib branch from 99cd145 to 7662dab Compare July 27, 2022 09:14
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-protot-ldeb-move--5h2aj9 July 27, 2022 09:14 Inactive
@lfdebrux lfdebrux force-pushed the ldeb-move-js-to-lib branch from 7662dab to 7da97d3 Compare July 27, 2022 09:24
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-protot-ldeb-move--5h2aj9 July 27, 2022 09:24 Inactive
@BenSurgisonGDS
Copy link
Contributor

I think the test can be fixed by replacing:

 ` expect.stringMatching(/.*\/VERSION.txt$/),`

with

  `expect.stringMatching(/.*[\/,//]VERSION.txt$/),`

@lfdebrux lfdebrux force-pushed the ldeb-move-js-to-lib branch from 7da97d3 to 0a17593 Compare July 27, 2022 09:58
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-protot-ldeb-move--gmtxvp August 2, 2022 12:15 Inactive
@lfdebrux lfdebrux force-pushed the ldeb-move-js-to-lib branch from ffe15f6 to e0ff427 Compare August 2, 2022 12:20
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-protot-ldeb-move--gmtxvp August 2, 2022 12:21 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-protot-ldeb-move--gmtxvp August 2, 2022 14:55 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-protot-ldeb-move--gmtxvp August 2, 2022 15:15 Inactive
@lfdebrux lfdebrux force-pushed the ldeb-move-js-to-lib branch from 7b7f1e9 to 364bfae Compare August 2, 2022 15:22
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-protot-ldeb-move--gmtxvp August 2, 2022 15:23 Inactive
@lfdebrux lfdebrux force-pushed the ldeb-move-js-to-lib branch from 364bfae to ca3f699 Compare August 2, 2022 15:25
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-protot-ldeb-move--gmtxvp August 2, 2022 15:25 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-protot-ldeb-move--gmtxvp August 2, 2022 16:25 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-protot-ldeb-move--gmtxvp August 2, 2022 17:04 Inactive
@lfdebrux
Copy link
Member Author

lfdebrux commented Aug 3, 2022

Note that we make sure that all pages include both scripts by adding kit.js to app/views/includes/scripts.html. This PR doesn't help users update that file, but ideally in another PR we would make it so that updating to v13 also takes care of this.

Just wondering if it should be part of this PR, given other changes are being made to the update scripts

As this PR is already pretty big I've put the layout template includes changes in a separate PR, #1499.

lfdebrux and others added 5 commits August 3, 2022 09:26
Also rename application.js to kit.js to avoid clashes.

Co-authored-by: Joe Lanman <joe.lanman@digital.cabinet-office.gov.uk>
Co-authored-by: NoraGDS <57447099+NoraGDS@users.noreply.github.com>
We want to make sure that any shared JS files we create can't have name
collisions with files the user wants to create. Putting the files in a
separate namespaced folder mitigates against this.
The update script tests run create-release-archive a lot, which involves
a lot of shelling out, and on Windows this is very slow, so we increase
the test timeout again to account for this. Ideally there would be less
shelling out, but that's a problem for another day.
@lfdebrux lfdebrux force-pushed the ldeb-move-js-to-lib branch from c07ecb9 to 5e5e10b Compare August 3, 2022 08:29
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-protot-ldeb-move--gmtxvp August 3, 2022 08:29 Inactive
@lfdebrux lfdebrux force-pushed the ldeb-move-js-to-lib branch from 5e5e10b to f88485e Compare August 3, 2022 08:38
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-protot-ldeb-move--gmtxvp August 3, 2022 08:39 Inactive
We want to make sure that on Windows we can compare files regardless of
what line endings the users files have, and that any changes we make
have the same line endings.
Simplify the update shell script by combining the Node.js post scripts
into one mega script.

We also move the final message from update script to very end.
@lfdebrux lfdebrux force-pushed the ldeb-move-js-to-lib branch from f88485e to 5e94bc7 Compare August 3, 2022 08:57
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-protot-ldeb-move--gmtxvp August 3, 2022 08:57 Inactive
Copy link
Contributor

@joelanman joelanman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great work

@lfdebrux lfdebrux merged commit 5da3de9 into main Aug 3, 2022
@lfdebrux lfdebrux deleted the ldeb-move-js-to-lib branch August 3, 2022 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Simplify update process - application.js
6 participants